Skip to content

Add intadd function #9742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add intadd function #9742

wants to merge 1 commit into from

Conversation

adsr
Copy link
Contributor

@adsr adsr commented Oct 14, 2022

Useful for when integer overflow is desired instead of becoming a float.

Synopsis:

php > var_dump(PHP_INT_MAX + 1);
float(9.223372036854776E+18)
php > var_dump(intadd(PHP_INT_MAX, 1));
int(-9223372036854775808)

Can write an RFC if desired.

(Requesting hacktoberfest-accepted label for Hacktoberfest cred.)

@juan-morales
Copy link
Contributor

Hello @adsr

Can write an RFC if desired.

is not desired, in your case ... is a must I would say.

bwoebi
bwoebi approved these changes Oct 14, 2022
View reviewed changes
Copy link
Member

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@juan-morales
Copy link
Contributor

@bwoebi Hello.

I Am curious about your approval on the PR what Does it mean?

This is just to know More about the process in PHP contribution.

iluuu1994
iluuu1994 reviewed Oct 14, 2022
View reviewed changes
ext/standard/math.c
Z_PARAM_LONG(addend2)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(addend1 + addend2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed integer overflow is undefined, this should be converted to unsigned (and back).

@cmb69
Copy link
Member

cmb69 commented Oct 14, 2022

IMO, this needs at least discussion on the internals mailing list. Personally, I'm not in favor of this, unless we'd implement full support for all operators (e.g. multiplication). And even then, I almost never find overflow behavior reasonable, and would rather prefer an exception (or way better, bigint support).

@juan-morales
Copy link
Contributor

I agree with @cmb69

@devnexen
Copy link
Member

Let's see what people think on the list but that s a no for me, usually I appreciate new ideas, but I m not liking this one.

@adsr adsr requested a review from bukka as a code owner October 7, 2023 13:51
@adsr adsr requested a review from kocsismate as a code owner April 15, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iluuu1994 iluuu1994 iluuu1994 left review comments

@bwoebi bwoebi bwoebi approved these changes

@bukka bukka Awaiting requested review from bukka bukka is a code owner

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner

Assignees
No one assigned
Labels
Extension: standard Feature hacktoberfest-accepted Status: Requires RFC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@adsr @juan-morales @cmb69 @devnexen @iluuu1994 @bwoebi @rlerdorf @Girgias